Skip to content

feat: Improve OpenShift provider connection validation using REST API #5074

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 4 commits into from
Jun 26, 2025
Merged

feat: Improve OpenShift provider connection validation using REST API #5074

merged 4 commits into from
Jun 26, 2025

Conversation

@talboren
Copy link
Member

close #5072

@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Jun 22, 2025
@vercel
Copy link

vercel bot commented Jun 22, 2025 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
keep ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 26, 2025 7:58am

@dosubot dosubot bot added API API related issues Provider Providers related issues labels Jun 22, 2025
cursor[bot]

This comment was marked as outdated.

Sign in to view

@dosubot dosubot bot added size:XXL This PR changes 1000+ lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Jun 23, 2025
cursor[bot]

This comment was marked as outdated.

Sign in to view

cursor[bot]

This comment was marked as outdated.

Sign in to view

@talboren talboren changed the title Improve OpenShift provider connection validation using REST API feat: Improve OpenShift provider connection validation using REST API Jun 23, 2025
@talboren talboren enabled auto-merge (squash) June 26, 2025 07:53
cursor[bot]
cursor bot reviewed Jun 26, 2025
View reviewed changes
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Duplicate Timezone in ISO 8601 Timestamp

The OpenshiftProvider generates malformed ISO 8601 timestamps for the kubectl.kubernetes.io/restartedAt annotation during rollout restarts. The code appends "Z" to a UTC datetime string that already includes a +00:00 timezone offset (e.g., 2025-06-15T10:30:00+00:00Z), resulting in an invalid format that combines both timezone indicators.

keep/providers/openshift_provider/openshift_provider.py#L457-L459

keep/keep/providers/openshift_provider/openshift_provider.py

Lines 457 to 459 in 5b6ae1d

k8s_client = self.__get_k8s_client()
now = datetime.datetime.now(datetime.timezone.utc)
now = str(now.isoformat("T") + "Z")

Fix in Cursor

Bug: Inconsistent Data Format in Event Retrieval

The __get_events method in OpenshiftProvider returns an inconsistent data format: raw event objects when sorting is successful, but dictionaries otherwise. This is exacerbated by the sorting logic's use of getattr(event, sort_by, None), which can introduce None values into the sorting key, leading to unpredictable behavior or errors if sort_by is an invalid attribute.

keep/providers/openshift_provider/openshift_provider.py#L286-L300

keep/keep/providers/openshift_provider/openshift_provider.py

Lines 286 to 300 in 5b6ae1d

if sort_by:
self.logger.info(f"Sorting events by {sort_by}")
try:
sorted_events = sorted(
events.items,
key=lambda event: getattr(event, sort_by, None),
reverse=True,
)
return sorted_events
except Exception:
self.logger.exception(f"Error sorting events by {sort_by}")
# Convert events to dict
return [event.to_dict() for event in events.items]

Fix in Cursor

Bug: Global SSL Warning Suppression Causes Issues

The OpenshiftProvider uses warnings.filterwarnings('ignore', message='Unverified HTTPS request') for SSL warning suppression. This global setting affects the entire application, potentially hiding critical SSL warnings from other components, instead of being scoped to the specific OpenShift connection test.

keep/providers/openshift_provider/openshift_provider.py#L113-L116

keep/keep/providers/openshift_provider/openshift_provider.py

Lines 113 to 116 in 5b6ae1d

# Suppress SSL warnings if insecure is True
if self.authentication_config.insecure:
# Suppress SSL verification warnings
warnings.filterwarnings('ignore', message='Unverified HTTPS request')

Fix in Cursor

Was this report helpful? Give feedback by reacting with 👍 or 👎

shahargl
shahargl approved these changes Jun 26, 2025
View reviewed changes
Copy link
Member

@shahargl shahargl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

@talboren talboren merged commit 0026346 into main Jun 26, 2025
22 checks passed
@talboren talboren deleted the cursor/fix-openshift-provider-connection-issue-4799 branch June 26, 2025 08:06
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jun 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cursor cursor[bot] cursor[bot] left review comments

@shahargl shahargl shahargl approved these changes

Assignees

No one assigned

Labels

API API related issues lgtm This PR has been approved by a maintainer Provider Providers related issues size:XXL This PR changes 1000+ lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[➕ Feature]: Add OpenShift provider connection

3 participants

@talboren @shahargl @cursoragent